fix(pbn): przepinaj osobę gdy PBN zmienił personId, zamiast ją pomijać#675
Open
mpasternak wants to merge 2 commits into
Open
fix(pbn): przepinaj osobę gdy PBN zmienił personId, zamiast ją pomijać#675mpasternak wants to merge 2 commits into
mpasternak wants to merge 2 commits into
Conversation
OsobaZInstytucji ma DWA klucze unikalne: personId (OneToOne na Scientist)
i polonUuid. To polonUuid — identyfikator z POL-onu — jest stabilną
tożsamością fizycznej osoby; personId PBN potrafi zmienić, np. po scaleniu
zdublowanych profili.
Import dopasowywał wiersz wyłącznie po personId, więc osoba wracająca pod
nowym personId leciała na INSERT i rozbijała się o unikalność polonUuid.
Handler łapał IntegrityError, raportował do Rollbara i POMIJAŁ osobę —
czyli nowa tożsamość PBN nigdy nie trafiała do bazy, a ten sam błąd wracał
przy każdym kolejnym imporcie (Rollbar #1523, bpp.ihit.waw.pl).
Teraz dopasowanie idzie najpierw po polonUuid, a personId jest zwykłym
polem do zaktualizowania. Zostaje raportowany tylko przypadek naprawdę
niejednoznaczny: nowy personId ma już swój wiersz z innym polonUuid —
scalenie dwóch tożsamości to decyzja o danych, nie poprawka techniczna.
Przy okazji znika kruche dopasowanie po treści komunikatu wyjątku
("polonUuid" in str(e)), które łapało też naruszenie NOT NULL dla osób
bez polonUuid i raportowało je jako "konflikt".
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
…rak polonUuid) Uwagi z self-review poprzedniego commita: - Rozszerzenie `except IntegrityError` na gołe łapanie było celowe (import całej kadry nie ma padać przez jedną osobę), ale komentarz twierdził, że trafia tam wyłącznie kolizja personId. Trafia tam też NOT NULL na polach przysłanych przez PBN jako null i każdy inny błąd integralności — a w Rollbarze wszystkie wyglądały identycznie. Dokładamy treść naruszonego ograniczenia do extra_data i prostujemy komentarz. - Osoba bez polonUuid: zamiast dowiadywać się o tym okrężnie przez IntegrityError na NOT NULL i raportować jako "konflikt tożsamości", wychodzimy od razu z czytelnym logiem. Przy okazji znika ścieżka, w której pusty string leciał ValueError-em z UUIDField i wywracał cały import (call site nie jest osłonięty). - Testy: dołożone asercje na przepięcie institutionId (na nim stoi scoping per-uczelnia), na stabilność pk (przepięcie, nie delete+create — FK deduplikator_autorow.main_osoba_z_instytucji ma SET_NULL) oraz round-trip wszystkich 9 pól. Obie asercje zweryfikowane mutacją kodu. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH
Member
Author
Poprawki po self-reviewReview wychwycił trzy rzeczy w pierwszym commicie — wszystkie naniesione w 1. Rozszerzone 2. Osoba bez 3. Testy nie pilnowały tego, co refaktor faktycznie ruszył. Dołożone:
Obie nowe asercje zweryfikowane mutacją kodu produkcyjnego — po usunięciu Świadomie NIE zrobione
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
OsobaZInstytucjima dwa klucze unikalne:personId(OneToOne naScientist) orazpolonUuid. TopolonUuid— identyfikator z POL-onu — jeststabilną tożsamością fizycznej osoby;
personIdPBN potrafi zmienić, np. poscaleniu zdublowanych profili.
Import dopasowywał wiersz wyłącznie po
personId, więc osoba wracająca podnowym
personIdleciała na INSERT i rozbijała się o unikalnośćpolonUuid:Handler łapał to, raportował do Rollbara i pomijał osobę — czyli nowa
tożsamość PBN nigdy nie trafiała do bazy, a ten sam błąd wracał przy każdym
kolejnym imporcie. Rollbar
#1523, instalacja
bpp.ihit.waw.pl.Rozwiązanie
Dopasowanie idzie najpierw po
polonUuid, apersonIdjest zwykłym polemdo zaktualizowania — wiersz zostaje przepięty na nowy identyfikator PBN.
Raportowany zostaje tylko przypadek naprawdę niejednoznaczny: nowy
personIdma już swój wiersz z innym
polonUuid. Scalenie dwóch tożsamości PBN todecyzja o danych, nie poprawka techniczna — import to pomija i idzie dalej,
zamiast się przerywać.
Przy okazji znika kruche dopasowanie po treści komunikatu wyjątku
(
"polonUuid" in str(e)), które łapało również naruszenieNOT NULLdla osóbbez
polonUuidi raportowało je jako „konflikt".Testy
Nowy
src/pbn_integrator/tests/test_osoba_z_instytucji.py— TDD: testprzepięcia najpierw padał (
_zapisz_osobe_z_instytucjizwracałoFalse).test_integrator_import.py+test_importer_authors.py—14 passed
🤖 Generated with Claude Code
https://claude.ai/code/session_01NcAqeqyqBzNEkkVnhpHDaH